Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix caret can disappear from script editor on Windows #93976

Conversation

Hilderin
Copy link
Contributor

@Hilderin Hilderin commented Jul 5, 2024

Fixes #92821
Also probably fixes #92634 but did not find a way to reproduce more than once.

This was not easy to reproduce! To reproduce I did Alt-Tab while resizing the editor windows. And even then, it takes multiple retries to reproduce the problem.

I’m pretty certain that the problem came from a situation where a new SetTimer was called before the timer was actually triggered resulting in a situation where activate_timer_id and move_timer_id had the same timer_id. Both SetTimer for activate_timer_id and move_timer_id had the same nIDEvent, so the second SetTimer replaced the first timer.

Documentation for SetTimer from Microsoft:
https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-settimer
image

With some debug print line, we can see that the activate timer can in some situations be created before the resize is triggered:
image

I did 2 modifications:

  • Changed the nIDEvent for activate_timer_id to 2.
  • When WM_ACTIVATE is received but activate_timer_id != 0, a new SetTimer is created anyway, replacing the first one. That should prevent the weird case where activate_timer_id was never triggered and resetted.

@Hilderin Hilderin requested a review from a team as a code owner July 5, 2024 15:53
@AThousandShips AThousandShips requested a review from a team July 5, 2024 15:58
@AThousandShips AThousandShips added this to the 4.3 milestone Jul 5, 2024
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave it some testing and it fixes all problems listed in the linked issues.

Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not important, but Instead of using hard-coded 1 and 2 it would be much better for readability (and to avoid future mistakes) to add an enum with IDs, somehting like:

enum TimerID {
    TIMER_ID_MOVE_REDRAW = 1,
    TIMER_ID_WINDOW_ACTIVATION = 2,
};

@Hilderin Hilderin force-pushed the fix-caret-disappear-from-script-editor branch from ba1cc62 to ebd1ab6 Compare July 8, 2024 11:13
@Hilderin
Copy link
Contributor Author

Hilderin commented Jul 8, 2024

Good idea, I pushed modifications to add the enum.

@akien-mga akien-mga merged commit 3220b6f into godotengine:master Jul 8, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

@akien-mga akien-mga changed the title Fix caret can disappear from script editor Fix caret can disappear from script editor on Windows Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Caret can disappear from script editor Scroll wrap in 2D editor sometimes doesn't work
5 participants